Skip to content

Conversation

aelovikov-intel
Copy link
Contributor

No description provided.

@aelovikov-intel aelovikov-intel force-pushed the ci-abi-compat branch 4 times, most recently from 30a6942 to 97b4425 Compare August 6, 2025 15:11
@aelovikov-intel aelovikov-intel merged commit efe036e into intel:sycl Aug 7, 2025
36 of 37 checks passed
@aelovikov-intel aelovikov-intel deleted the ci-abi-compat branch August 7, 2025 18:30
aelovikov-intel added a commit that referenced this pull request Aug 7, 2025
#19719 broke `workflow_dispatch` mode
for manual E2E task invocation for compatibility testing as we don't
have the required `binaries_artifact` input in that mode. Github limits
number of input parameters in `workflow_dispatch` mode, so restore the
functionality without introducing extra input.
aelovikov-intel added a commit that referenced this pull request Aug 7, 2025
#19719 broke `workflow_dispatch` mode
for manual E2E task invocation for compatibility testing as we don't
have the required `binaries_artifact` input in that mode. Github limits
number of input parameters in `workflow_dispatch` mode, so restore the
functionality without introducing extra input.
aelovikov-intel added a commit that referenced this pull request Aug 7, 2025
#19744)

#19719 broke `workflow_dispatch` mode
for manual E2E task invocation for compatibility testing as we don't
have the required `binaries_artifact` input in that mode. Github limits
number of input parameters in `workflow_dispatch` mode, so restore the
functionality without introducing extra input.

# Need more investigation by the author:

# https://github.com/intel/llvm/pull/18314
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alexandr-Konovalov , looks like your #18314 broke backward compatibility here, see
https://github.com/intel/llvm/actions/runs/16882134385 (using nightly-2025-05-08, pass)
https://github.com/intel/llvm/actions/runs/16882361500 (using nightly-2025-05-09, fail)
Please investigate and either fix (might need backporting to sycl-rel-6_3) or create a PR updating this file describing why it wasn't an ABI-break or was allowed/desired/approved.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, an interesting issue! Opened [SYCL] Add copy/move ctors and assignment operators to sycl::detail::optional #19775.

Assert/assert_in_simultaneously_multiple_tus.cpp
Assert/assert_in_simultaneously_multiple_tus_one_ndebug.cpp

# https://github.com/intel/llvm/pull/17442
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@steffenlarsen , looks like your #17442 broke backward compatibility here, see
https://github.com/intel/llvm/actions/runs/16882650449 (using nightly-2025-05-01, pass)
https://github.com/intel/llvm/actions/runs/16882716814 (using nightly-2025-05-02, fail)
Please investigate and either fix (might need backporting to sycl-rel-6_3) or create a PR updating this file describing why it wasn't an ABI-break or was allowed/desired/approved.

Copy link
Contributor

@steffenlarsen steffenlarsen Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do! Great addition, by the way!

See #19764.

NewOffloadDriver/split-per-source-main.cpp
NewOffloadDriver/sycl-external-with-optional-features.cpp

# https://github.com/intel/llvm/pull/18277
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igchor , looks like your #18277 broke backward compatibility here, see

https://github.com/intel/llvm/actions/runs/16883391382 (using nightly-2025-05-19, pass)
https://github.com/intel/llvm/actions/runs/16883503382 (using nightly-2025-05-23, fail)
Please investigate and either fix (might need backporting to sycl-rel-6_3) or create a PR updating this file describing why it wasn't an ABI-break or was allowed/desired/approved.

Comment on lines +74 to +75
# https://github.com/intel/llvm/pull/19328
Adapters/interop-level-zero-buffer-ownership.cpp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igchor , looks like your #19328 broke backward compatibility here, see

https://github.com/intel/llvm/actions/runs/16883683175 (using nightly-2025-07-10, pass)
https://github.com/intel/llvm/actions/runs/16883690547 (using nightly-2025-07-11, fail)
Please investigate and either fix (might need backporting to sycl-rel-6_3) or create a PR updating this file describing why it wasn't an ABI-break or was allowed/desired/approved.

Comment on lines +56 to +68
# https://github.com/intel/llvm/pull/18403 (pulldown, so probably offload-tools
# team should be looking into this)
DeviceImageDependencies/NewOffloadDriver/dynamic.cpp
DeviceImageDependencies/NewOffloadDriver/free_function_kernels.cpp
DeviceImageDependencies/NewOffloadDriver/math_device_lib.cpp
DeviceImageDependencies/NewOffloadDriver/objects.cpp
DeviceImageDependencies/NewOffloadDriver/singleDynamicLibrary.cpp
NewOffloadDriver/aot-gpu.cpp
NewOffloadDriver/buffer.cpp
NewOffloadDriver/multisource.cpp
NewOffloadDriver/spirv_device_obj_smoke.cpp
NewOffloadDriver/split-per-source-main.cpp
NewOffloadDriver/sycl-external-with-optional-features.cpp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@intel/dpcpp-tools-reviewers , looks like #18403 pulldown broke backward compatibility here

https://github.com/intel/llvm/actions/runs/16883874414 (using nightly-2025-05-16, pass)
https://github.com/intel/llvm/actions/runs/16883880550 (using nightly-2025-05-17, fail)
Please investigate and either fix (might need backporting to sycl-rel-6_3) or create a PR updating this file describing why it wasn't an ABI-break.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was suggested to me to tag @asudarsa here.

Comment on lines +18 to +24
# https://github.com/intel/llvm/pull/18565
# This one should have probably be done in multiple PRs, first improving the
# test's CHECKs and then making an actual functional change.
#
# Based on the title, I'd expect to see reduction in number of ur*retain/release
# calls, but it only moves a few ur*release CHECKs, so I'll let the author
# update this explanation.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alexandr-Konovalov , please upload a PR that updates this comment with explanation how the change wasn't ABI-breaking.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened [CI] Add clarification for KernelAndProgram/disable-caching change #19822

# binaries built with sanitizers?
Sanitizer

# https://github.com/intel/llvm/pull/19238
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndreiZibrov , please upload a PR that updates this with an explanation why this was desired (I'd expect something along the lines of allowing ABI breaks in an experimental extension).

Also tagging @AlexeySachkov

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aelovikov-intel - Do you have the output of this failure at hand?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Clarification here: #19774

Comment on lines +35 to +38
# https://github.com/intel/llvm/pull/17955, experimental extension
AsyncAlloc/device/async_alloc_from_pool.cpp
AsyncAlloc/device/async_alloc_zero_init.cpp
AsyncAlloc/device/ooo_queue_async_alloc_from_pool.cpp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@intel/sycl-graphs-reviewers , can you please check if that break was intentional and if so update the comment here?

Using 2025-06-18 (pass): https://github.com/intel/llvm/actions/runs/16884717829
Using 2025-06-19 (fail): https://github.com/intel/llvm/actions/runs/16884725131

ggojska pushed a commit to ggojska/llvm that referenced this pull request Aug 12, 2025
intel#19744)

intel#19719 broke `workflow_dispatch` mode
for manual E2E task invocation for compatibility testing as we don't
have the required `binaries_artifact` input in that mode. Github limits
number of input parameters in `workflow_dispatch` mode, so restore the
functionality without introducing extra input.
@xtian-github
Copy link

@cdai2 Please take a look this PR to see what is impact on Sanitizer compatibility.

aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Aug 18, 2025
intel#19719 and
intel#19761 added pre-commit jobs to run
E2E tests pre-built with latest "open-source" releases against the newly
built sycl-toolchain libraries. Those can fail if either an actual break
is happenning or if the test was doing some `FileCheck`ing and that
output has changed in some way (which might not be an actual ABI break).

However, I think the testing is still good enough to require an explicit
approvals by folks in charge of ABI breaking changes. For the case of
just output change the author should be able to convince owners that the
change isn't ABI-breaking relatively easily.
aelovikov-intel added a commit that referenced this pull request Aug 18, 2025
…19820)

#19719 and
#19761 added pre-commit jobs to run
E2E tests pre-built with latest "open-source" releases against the newly
built sycl-toolchain libraries. Those can fail if either an actual break
is happenning or if the test was doing some `FileCheck`ing and that
output has changed in some way (which might not be an actual ABI break).

However, I think the testing is still good enough to require an explicit
approvals by folks in charge of ABI breaking changes. For the case of
just output change the author should be able to convince owners that the
change isn't ABI-breaking relatively easily.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants